Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configuration to allow overriding the encoding method. #23

Closed

Conversation

FabienChaynes
Copy link
Contributor

This PR is related to #22.
It is obviously missing some specs and documentation, which I will add once we'll agree on the implementation.

It adds a Configuration object with an @encoding_method variable.

This variable will receive a Proc and will default to the ::URI.encode_www_form method. If the variable is set, the Proc should expect a single argument responding to #to_h and return a string of the encoded hash.
It'd allow users to override the method encoding data in HTTP::FormData::Urlencoded#initialize.

An example of the usage could be:

HTTP::FormData.configure do |config|
  custom_encode_www_form = lambda do |enum|
    unescaped_chars = /[^a-z0-9\-\.\_\~]/i
    enum.map do |k, v|
      if v.nil?
        ::URI::DEFAULT_PARSER.escape(k.to_s, unescaped_chars)
      elsif v.respond_to?(:to_ary)
        v.to_ary.map do |w|
          str = ::URI::DEFAULT_PARSER.escape(k.to_s, unescaped_chars)
          unless w.nil?
            str << '='
            str << ::URI::DEFAULT_PARSER.escape(w.to_s, unescaped_chars)
          end
        end.join('&')
      else
        str = ::URI::DEFAULT_PARSER.escape(k.to_s, unescaped_chars)
        str << '='
        str << ::URI::DEFAULT_PARSER.escape(v.to_s, unescaped_chars)
      end
    end.join('&')
  end

  config.encoding_method = custom_encode_www_form
end

@ixti
Copy link
Member

ixti commented May 30, 2018

I don't like the idea of configuration object - it's overkill in here IMO. Also, I think there should be a way to pass encoder to Urlencoded initializer.

@FabienChaynes FabienChaynes force-pushed the 22-custom-encoding-method branch from 93d42e1 to 74ecde7 Compare May 30, 2018 08:46
@FabienChaynes
Copy link
Contributor Author

I removed the configuration in the first commit and allowed Urlencoded to receive the encoder at initialization in the second one.
Now it will allow a simpler monkey patching along the lines of:

require "uri"

module HTTP
  module FormData
    def self.encoder
      lambda do |enum|
        unescaped_chars = /[^a-z0-9\-\.\_\~]/i
        enum.map do |k, v|
          if v.nil?
            ::URI::DEFAULT_PARSER.escape(k.to_s, unescaped_chars)
          elsif v.respond_to?(:to_ary)
            v.to_ary.map do |w|
              str = ::URI::DEFAULT_PARSER.escape(k.to_s, unescaped_chars)
              unless w.nil?
                str << '='
                str << ::URI::DEFAULT_PARSER.escape(w.to_s, unescaped_chars)
              end
            end.join('&')
          else
            str = ::URI::DEFAULT_PARSER.escape(k.to_s, unescaped_chars)
            str << '='
            str << ::URI::DEFAULT_PARSER.escape(v.to_s, unescaped_chars)
          end
        end.join('&')
      end
    end
  end
end

I'm not a huge fan of this solution though 😕
Do you have something else in mind to customize this without a configuration object?

@FabienChaynes
Copy link
Contributor Author

Re reading your comment, maybe you were just talking about exposing the encoder without wrapping it inside a Configuration object (which I did in d7a2c3f).
This way it'd be used like this:

HTTP::FormData.encoder = lambda do |enum|
  # ...
end

@ixti
Copy link
Member

ixti commented May 31, 2018

I have changed this code a bit and merged this PR as 3bf2e83

So basically I have moved away from the idea of having per-form encoder (at least for now). And exposed encoder on Urlencoded class. Meaning one can override default implementation with:

HTTP::FormData::Urlencoded.encoder = lambda { |data| ... }

@ixti ixti closed this May 31, 2018
@ixti
Copy link
Member

ixti commented May 31, 2018

Thank you!

@FabienChaynes
Copy link
Contributor Author

Great, thanks!

@ixti
Copy link
Member

ixti commented Jun 1, 2018

Released as v2.1.1

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 23, 2018
## 2.1.1 (2018-06-01)

* [#23](httprb/form_data#23)
  Allow override urlencoded form data encoder.
  [@FabienChaynes][]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants